Delete user if inactive only #61
Conversation
Luis-3M
changed the title
|
@ChrisPates could you review this PR pls ? |
|
@ChrisPates What are your thoughts on this code optimisation? |
|
So I have a number of issues to address around how the adds/updates/deletes are determined. The part that is really the source of this undesirable behaviour is the call to retrieve deleted users from the Google side. So I'm looking at performing a full comparison (which wasn't possible before the introduction of the identityStore api) and potentially making use of the external identifier fields in identity store so matching is more accurate. Also this would provide a route back from the use of DisplayName which allows duplicates on the Google side (since they key on primary email address) but on the identity store side it must because it keys on displayName. |
|
Awesome looking forward to it @ChrisPates - indeed relying on just the users list API call on the Google side (with Will keep an eye on this thread. |
|
I've had to update the workflows, can you update your fork from master and update this PR, I'll get it merged into the main code base. |
|
Thanks @ChrisPates - I'll get this done as soon as possible. |
Issue #59
Description of changes:
Problem
Noticed that if we delete a user on the Google side and then recreate it with the exact same details the following occurs:
ssosync will consider that user as inactive based on the API response, hence deleting it from AWS SSO
https://github.com/awslabs/ssosync/blob/master/internal/sync.go#L70-L103
https://github.com/awslabs/ssosync/blob/master/internal/google/client.go#L64-L73
ssosync will then list all the active users and it will see that it needs to add again the user that has just been deleted in the previous step.
https://github.com/awslabs/ssosync/blob/master/internal/sync.go#L105-L152
At this stage user will have no permission sets assigned on AWS SSO, and this will happen every time we run ssosync.
Possible Solution
I believe that it would be beneficial to change the
SyncUsersfunction so it checks that all of the API returned deleted users are in fact inactive, and for the ones that are now active it just ignores them by building a new list of deleted users which we can iterate over just like its being done atm.